-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add async support to babel-jest #11192
Conversation
Deploy preview for jestjs ready! Built without sensitive environment variables with commit 6e4c1c7 |
Deploy preview for jestjs ready! Built without sensitive environment variables with commit 809380d |
packages/babel-jest/src/index.ts
Outdated
filename: Config.Path, | ||
transformOptions: JestTransformOptions, | ||
): PartialConfig { | ||
const {cwd} = transformOptions.config; | ||
// `cwd` first to allow incoming options to override it | ||
const babelConfig = loadPartialConfig({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is also the async version of this function, and I think it allows using await
in config files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uh it's because the types are not maintained by us.
We are slowly converting our codebase to TS, we'll release official type definitions when we finish.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can send a PR to DT 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Codecov Report
@@ Coverage Diff @@
## master #11192 +/- ##
==========================================
- Coverage 64.28% 64.21% -0.08%
==========================================
Files 307 308 +1
Lines 13458 13480 +22
Branches 3285 3286 +1
==========================================
+ Hits 8652 8656 +4
- Misses 4100 4117 +17
- Partials 706 707 +1
Continue to review full report at Codecov.
|
import babelJest from 'babel-jest'; | ||
|
||
export default { | ||
...babelJest.default.createTransformer({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not too happy about this one 🙁 We should output ESM from Jest 28 or something
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think adding export { createTransformer }
to babel-jest would be enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep that'd work, but "real" ESM should use a single default
export of an object, so the issue is a "fake" esm module imported directly in native ESM. Edge casey enough that I don't think it's worth changing before just using native ESM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, and just to be explicit - the .default
change is not because of this PR, but the change made in #11193
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
Why not 🙂 Only transformer we ship in Jest.
/cc @nicolo-ribaudo
Test plan
Not sure... I've verified via some inserted
console.log
s that it works. Not sure how to write a test. (marked as draft due to this)EDIT: copied the synchronous test 🤷 Also added an e2e test